BDMS-285: Well Inventory CSV validation/upload#145
Conversation
|
|
||
| // Helper to convert field name to display name | ||
| // Converts snake_case to Title Case (e.g., "well_name_point_id" -> "Well Name Point Id") | ||
| const getDisplayName = (fieldName: string): string => { |
There was a problem hiding this comment.
This helper looks good, but it might make sense to move it into its own small util file (e.g., snakeToTitleCase.ts) so it can be reused wherever needed.
You may also want to add a guard to handle fieldName or name being null or undefined, or update the type signature to NonNullable<string> so the function guarantees it always receives a valid string. Adding a fallback return value or throwing a clear error will make this more robust and help avoid silent failures. Below is a code suggestion that includes a safety guard and a helpful function name change.
| const getDisplayName = (fieldName: string): string => { | |
| export const snakeToTitleCase = (name: string | null | undefined): string => { | |
| if (!name) return ''; | |
| return name | |
| .trim() | |
| .split('_') | |
| .filter(Boolean) | |
| .map(w => w.charAt(0).toUpperCase() + w.slice(1)) | |
| .join(' '); | |
| }; |
Finally, the transformation logic is straightforward, but you could make it slightly safer and more concise by trimming input and ignoring extra underscores (e.g., .filter(Boolean), which removes an additional space if the field name has two or more underscores), or by handling unexpected characters.
There was a problem hiding this comment.
Yeah I think I will just remove this helper. It seems best to keep the field name on the table matching the field name on the csv so there is no confusion. And it reduces the need for text manipulation.
| </Button> | ||
| </label> | ||
| <Link | ||
| href="https://docs.google.com/spreadsheets/d/1USYvb-A_-jsdY3qoPQBdkfjv5jjjYGaympp8SMAqJDo/edit?usp=sharing" |
There was a problem hiding this comment.
Is it possible that Google will change this link in the future, rendering it dead?
There was a problem hiding this comment.
There is a chance it would change, but this seems like a straightforward way for now. But open to other ideas.
There was a problem hiding this comment.
After digging deeper into this, it seems that Google keeps their links quite stable. The FILE_ID is unlikely to change even if you modify the file's name, location, or permissions. If you want to prevent users from accidentally editing the file, you could update the link to a read-only format, like this: https://docs.google.com/spreadsheets/d/FILE_ID/view. Other than that, this approach looks good.
|
This is more proof that it successfully created a new row in my local DB while testing this PR. |
…ent with csv file
… multiple files somehow.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| cy.intercept('POST', '**/well-inventory-csv', (req) => { | ||
| // For data sent as multipart string, check body contains CSV content | ||
| const body = req.body as string | ||
| const originalLines = originalCsv.split('\n') | ||
|
|
There was a problem hiding this comment.
Cypress intercept misreads multipart payload
The upload flow posts a FormData (see handleSubmit creating a File and appending it to FormData), but the new Cypress test assumes req.body is a CSV string and immediately casts it to string. For multipart requests req.body is a FormData/Blob, so the include assertions will run against "[object FormData]" and always fail even when the CSV is sent correctly. This makes the added test a permanent false negative. Consider extracting the blob from req.body (e.g., req.body.get('file') and reading it) before asserting on its contents.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I originally tried this approach, but the Cypress interceptor does not allow the .get('file'), the submission is just sent as a large string in the body in the Cypress environment, from what I can tell.
TylerAdamMartinez
left a comment
There was a problem hiding this comment.
overall, looks good to me
|
Closing this PR after the product discussion on Wednesday: getting data in > having a front end for getting data in. Keep this branch but focus work on CLI DataIntegrationGroup/OcotilloAPI#284 |


Why
This PR addresses the following problem / context:
How
Implementation summary - the following was changed / added / removed:
Notes
Any special considerations, workarounds, or follow-up work to note?
🚨 ⚠️ 💀 [NO TICKET]: Well inventory csv Feature Tests OcotilloAPI#247
csv_upload.mp4